-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(publish): Avoid errors when files are ignored by git #2445
Conversation
You need to add tests. Merely passing existing ones simply exposes how shoddy the existing test coverage is. |
@evocateur I've also added a change and tests to the git-checkout command because we found that was also erroring out for the same reason on the githead edits. So that should be clearing out correctly now. |
Would really love to see this added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the test!
Why wasn't this marked as a breaking change? Definitely broke our process because we only expected lerna to commit the changes that it made and no others. Files were generated/copied around as part of our build process and specifically weren't committed, and they have now been published and pushed back to source after this change. Previously these files were just ignored because they weren't explicitly added, but now we've only just noticed that we have extra CI files floating around in our git repo and published packages for the past few days. Now the reason why our process is like that is questionable in itself :) We shouldn't be copying files into packages like that but we also didn't expect the behavior to change in a significant way like this, or at least be able to opt-into the breaking change and handle it accordingly by properly gitignoring the files. I just hope that someone doesn't have something with credentials or secrets checked into their packages/public repos now. |
😬 This happened to me. My |
Same problem here 😞 Adding files outside "Lerna scope" seems a bit dangerous... Maybe parsing the .gitignore and then filtering the files array fits better given the original bug description? I could take care of it if you agree @evocateur 👍 |
I did not deem this a major because I thought people with secrets on their filesystem would be prudent enough to add them to their Guess I'll stick it behind an option, now. Struggling to name it, though... |
think i'm gonna go with (the default will be flipped in the next major, preserving pre-3.22.0 behavior for the rest of the 3.x range) |
(definition of pathspec, for those interested in git terminology) |
I won't personally stretch those cases to secrets only. Someone with local changes who's not aware of this behavior could execute lerna version command without removing those changes. Moreover, in some scenarios, you could edit something just for manual testing purposes locally before releasing a new version, in order to verify a feature accomplishes its proposal. Thanks for your time and patience🤘 ! |
@evocateur although there is currently the option |
Change
git-add
command to use the '.' syntax so as only to add files that aren't in the .gitignore that have been changed.Description
Instead of adding files by their specific names (which would cause git to complain if one of those files was included in the .gitignore file), we use the
git add .
syntax to smartly grab any files that have been changed.Motivation and Context
This fixes issue #2151 , where if you have projects that you are entirely generating and want to be able to publish without commiting to your github repo, you are now able to do so.
How Has This Been Tested?
This has been run through the lerna tests successfully and it was tested in practice against my repo with a similar use-case to the one listed above with successful results.
Types of changes
Checklist:
I don't think this needs new tests since the git-add tests still cover the functionality that it's supposed to accomplish. And while it might need documentation I wouldn't be sure where it should be placed so if anyone has input on those two, I'd be happy to update accordingly.